Skip to content

fix: Tracer.logConnectionDetails defaults to true#80

Merged
obitech merged 4 commits into
exaring:mainfrom
trygve-baerland:main
May 21, 2026
Merged

fix: Tracer.logConnectionDetails defaults to true#80
obitech merged 4 commits into
exaring:mainfrom
trygve-baerland:main

Conversation

@trygve-baerland

Copy link
Copy Markdown
Contributor

From my understanding, the Tracer.logConnectionDetails is meant to default to true (going by how the config struct is initialized in NewTracer), and using WithDisableConnectionDetailsInAttributes to set it to false.

This PR is a fix so the tracer behaves as described above.

I've also added a table of tests making assertions about what attributes are present in traces under different configurations. Since these tests set up a mock pg connection, the changes to tests are quite verbose. I'll be more than happy to get any suggestions on better ways of testing the attributes behaviour (or any other suggestions, for that matter).

@trygve-baerland

Copy link
Copy Markdown
Contributor Author

I see now that this was previously solved by #69, but looks to have been inadvertently reverted by #67.

Sorry for not doing my due diligence before creating this PR.

@obitech

obitech commented May 13, 2026

Copy link
Copy Markdown
Member

Oh boy, thanks for catching this (again). Could you potentially rebase your PR onto #79 and refactor the tests to use testify? I expect 79 to land soon and will merge yours right after. Thank you!

@trygve-baerland

Copy link
Copy Markdown
Contributor Author

No problem, @obitech, I can do that.

@obitech obitech left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@obitech obitech merged commit 626246c into exaring:main May 21, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants